Repository navigation
feat: add runtime memory admission for detached compilers - #803
ZheFeng7110 wants to merge 16 commits into
Conversation
An ordinary compile edge runs through `mcpp compile-admit` now, and ninja echoes that wrapper as the command that failed. `failed_command_in` required a clang driver as the FIRST word of the line, found none, and the note for a compiler that does not support C++20 coroutines on its MSVC ABI stopped appearing — the failure was left unexplained. That is windows-e2e 892 case C: a C++20 program that uses coroutines, built for i686-windows-msvc, exits non-zero with clang's two errors and no note (run 38268962914, windows-e2e 2/3). Cases A and B kept passing because the std module precompile carries its command in mcpp's own message, which no wrapper touches. The driver is now searched for wherever it sits on the line; the target and the standard read from that line are unchanged by the wrapper. Two tests carry the CI output as it arrives through the wrapper, and fail without this.
|
Review: #803 — memory admission control Verdict: Request changes The feature is coherent (config → plan → ninja emission → compile-admit wrapper → shared token/memory gate), docs are accurate, and tests cover the new capacity Blockers
Major
Minor
Sound
Fix §1–§3 before merge; §4–§6 should follow in the same PR since they're all in the touched code paths. |
Address review item §1 by backing up the BMI before compilation and restoring its original timestamp when the successful output is equivalent. Cover the wrapper and downstream Ninja restat behavior with regression tests.
Address review item §2 by removing counting slots from ordinary and two-phase compile admission. Retain memory gating with a shared single-slot fallback, include memory waits in timeout accounting, and reclaim stale fallback tokens for every build strategy. Update regression coverage and both language manuals.
Address review item §3 by rounding observed available memory up to a 4 GiB bucket, retaining the unknown-memory fallback and saturating at the integer limit. Add boundary coverage and document upward quantization in both language manuals.
Reject a malformed --cap, --mem-baseline or --mem-threshold instead of letting it fall back to the default silently: from_chars was unchecked, so a garbage value was indistinguishable from an omitted one. The check is shared by compile-admit and bmi-compile, and MCPP_MEM_BASELINE is validated the same way. Name the file when the scanner depfile cannot be read or written. The wrapper exited 1 with no output after a successful compile, which reads as an anonymous compiler failure. Cover both with tests/scripts/test_compile_admit.py.
The generated rules spelled the token directory as a literal in the admission prefix, as the $sched_sem edge variable and in the stale-token reclaim. Derive all three from one file-scope constant so they cannot drift apart.
compile-admit starts a child through cmd.exe on Windows (run_exec's documented shell path), where a literal newline ends the command. The pressure test's holding child embedded a newline, so only its first line ran: the child touched `ready` and exited at once, releasing the token the test needs it to hold. Whether that showed as a failure or as the "memory metrics unavailable" skip was then a ~10ms race — and the test never actually exercised admission on Windows. Make the program single-line (the wait loop moves into an exec'd string, whose newline is an escape rather than a separator) and require the holding edge to still be running before the gate is read, so an early exit is a named failure instead of a silent skip.
This branch has not been deployed
1 participant
Summary
Implements the first stage of #802 by adding runtime physical-memory admission control to the GCC
bmi_schedule = "on"detached compiler path.mcpp.platform.capacity.MemAvailableand cgroup v2 limits.build.memory_threshold_percent, defaulting to80.MCPP_MEMORY_THRESHOLD_PERCENTas an environment override.Implementation plan
Current status
Completed in this PR:
bmi-compilecommand.Not completed in this PR:
This PR is intentionally scoped as the first implementation stage. The remaining items should be addressed before treating #802 as fully implemented.
Tests
mcpp build --releasepassed before the final test rebuild.mcpp test capacitypassed: 11 tests passed.mcpp test -p platformpassed: 2 tests passed.git diff --checkpassed.Related issue
Closes #802